-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate reflection probes to required components #15737
Migrate reflection probes to required components #15737
Conversation
#[deprecated( | ||
since = "0.15.0", | ||
note = "Use the `LightProbe` and `EnvironmentMapLight` components instead. Inserting them will now also insert the other components required by them automatically." | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Do we actually want to deprecate this yet, considering you still need to add both components manually? Or should I only remove the spatial_bundle
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely unfortunate that the EnvironmentMapLight
component is also used on camera entities. Looking at the examples it feels like that should be merged into the Skybox
component. We could then require LightProbe
for EnvironmentMapLight
, but alas.
I think we should deprecate this, even though it's not perfect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by "also used on camera entities". Where else other than camera entities is it used?
Co-authored-by: Tim Blackbird <[email protected]>
Head branch was pushed to by a user without write access
@alice-i-cecile attempt number two? |
Objective
Getting closer to the end! Another part of the required components migration: reflection probes.
Solution
As per the proposal added by Cart (Proposal 2), make
LightProbe
requireTransform
andVisibility
, and deprecateReflectionProbeBundle
.Note that this proposal wasn't officially blessed yet, but it is the only existing one that really works, so I implemented it here for consideration.
Testing
I ran the reflection probe example, and it appears to work.
Migration Guide
ReflectionProbeBundle
has been deprecated in favor of inserting theLightProbe
andEnvironmentMapLight
components directly. Inserting them will now automatically insertTransform
andVisibility
components.